Skip to content

Implement xDS virtual host routing - #6322

Merged
jrhee17 merged 5 commits into
line:mainfrom
jrhee17:feat/xds-vhost-routing
Jul 30, 2025
Merged

Implement xDS virtual host routing#6322
jrhee17 merged 5 commits into
line:mainfrom
jrhee17:feat/xds-vhost-routing

Conversation

@jrhee17

@jrhee17 jrhee17 commented Jul 23, 2025

Copy link
Copy Markdown
Contributor

Motivation:

This changeset attempts to implement routing based on virtual hosts.

The relevant documentation regarding matching can be found in the following locations:

The rules for domain matching can be found in detail at the VirtualHost#domains section.

The relevant upstream implementation can be found in the router configuration:

Note that there is a slight difference in behavior compared to upstream - given that armeria's integration acts more as a client rather than a proxy, the default virtual host will be used when the authority is null.

Modifications:

  • Introduced VirtualHostMatcher which pre-computes a map for quicker virtual host matching.
  • RouteEntry#index, VirtualHost#index are exposed publicly. This allows for easier referencing to the selected route.

Result:

  • xDS clients route requests based on the authority

@jrhee17 jrhee17 added this to the 1.33.0 milestone Jul 23, 2025
@codecov

codecov Bot commented Jul 23, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (8150425) to head (e1eefe9).
⚠️ Report is 131 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #6322       +/-   ##
============================================
- Coverage     74.46%       0   -74.47%     
============================================
  Files          1963       0     -1963     
  Lines         82437       0    -82437     
  Branches      10764       0    -10764     
============================================
- Hits          61385       0    -61385     
+ Misses        15918       0    -15918     
+ Partials       5134       0     -5134     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jrhee17
jrhee17 marked this pull request as ready for review July 24, 2025 00:53

@minwoox minwoox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some questions. Looks good to me. 👍

if (routeEntries.isEmpty()) {
return null;
}
final RouteEntry routeEntry = routeEntries.get(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: The first route is only used for now, and it's going to be changed in the future. Is that correct?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, do you prefer route matching is done in the same PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I just wanted to make sure. 😉

for (VirtualHostSnapshot virtualHostSnapshot: routeSnapshot.virtualHostSnapshots()) {
for (String domain: virtualHostSnapshot.xdsResource().resource().getDomainsList()) {
domain = Ascii.toLowerCase(domain);
if ("*".equals(domain)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use the DomainMappingBuilder?

@jrhee17 jrhee17 Jul 28, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed 2ad3b39 so you can check failing tests.
https://github.com/line/armeria/actions/runs/16557827889

Most notably:

  • The behavior regarding asterisks is different. Single asterisks aren't allowed, and asterisks not separated by a dot aren't allowed.
  • Support for longest match for multiple matching entries isn't supported.

Even if DomainMappingBuilder is used, my feeling is that there is no benefit as most of the logic will have to be rewritten anyways.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ikhoon ikhoon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! 👍 👍

@minwoox minwoox left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 👍 👍

@jrhee17
jrhee17 merged commit 2441b44 into line:main Jul 30, 2025
13 of 14 checks passed
ikhoon pushed a commit that referenced this pull request Aug 4, 2025
Motivation:

Following #6322, this changeset
attempts to implement route matching (`Route#match`).
ref:
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-msg-config-route-v3-route

Implementation is focused on following upstream behavior as closely as
possible.
The corresponding implementation can be found here:
https://github.com/envoyproxy/envoy/blob/4bb05db55ecd583b6f451d81445d94210445176f/source/common/router/config_impl.cc#L831-L881

Matching rules that aren't implemented are:
- tls_context: Since the client doesn't act as a proxy yet, there are no
downstream tls contexts.
- dynamic_metadata: While this may be useful, users don't have a way to
supply a metadata directly for a request yet.
- runtime_fraction: This will be implemented when rtds is implemented.
- filter_state: may consider this when the set_filter_state filter is
introduced.
Overall, the above rules have been verified to not being used in basic
istio cases.

Modifications:

- Implemented `RouteEntryMatcher` which matches a route based on the
path, headers and query parameters.
- Added a dependency to `re2j`, which provides linear time regex
matching.
- ref:
https://www.envoyproxy.io/docs/envoy/latest/api-v3/type/matcher/v3/regex.proto#envoy-v3-api-msg-type-matcher-v3-regexmatcher
- Note that the default regex engine is `re2j` in
`bootstrap#default_regex_engine`
-
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/bootstrap/v3/bootstrap.proto
- Misc) Added CLAUDE.md to `.gitignore` since it seems like other major
repositories aren't checking in this file either.

Result:

- The xDS integration can now route requests based on the path, headers,
and query parameters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants